Reviewed 23 of 23 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @dzmitryhil, @miladz68, and @wojtek-coreum)

integration-tests/coreum/contract_client_test.go line 367 at r1 (raw file):

	require.NotNil(t, operation.OperationType.TrustSet)

	rejectTxEvidenceTrustSet := coreum.XRPLTransactionResultTrustSetEvidence{

nit: rejectedTxEvidenceTrustSet

integration-tests/coreum/contract_client_test.go line 428 at r1 (raw file):

	require.True(t, coreum.IsXRPLTokenNotEnabledError(err), err)

	_ = activeCurrency

do we need this ?

integration-tests/processes/send_from_xrpl_to_coreum_test.go line 159 at r1 (raw file):

	require.NoError(t, err)

	// fund owner to cover registration fees

nit: cover issuance fee ?

relayer/coreum/contract.go line 648 at r1 (raw file):

// GetXRPLToken returns an XRPL registered token or nil.
func (c *ContractClient) GetXRPLToken(ctx context.Context, issuer, currency string) (*XRPLToken, error) {
	tokens, err := c.GetXRPLTokens(ctx)

question:
Does this query return all tokens or enabled only?

relayer/processes/amount.go line 18 at r1 (raw file):

	// XRPLIssuedCurrencyDecimals is XRPL decimals used the on coreum.
	XRPLIssuedCurrencyDecimals = 15
	// XRPIssuer is XRP issuer name used the on coreum.

nit: WDYM used the on coreum ?

on the coreum probably ?

Also we don't use this amount and currency on coreum these values are used in XRPL, no ?

relayer/processes/amount.go line 54 at r1 (raw file):

		xrplValue, err := rippledata.NewValue(amountString, true)
		if err != nil {
			return rippledata.Amount{}, errors.Wrapf(err, "failed to convert amount stringy to ripple.Value, amount stirng: %s", amountString)

nit: amount stirng:

-> string

relayer/processes/xrpl_tx_observer.go line 171 at r1 (raw file):

// IsExpectedSendEvidenceError returns true is error is cause of the re-submitting of the transaction.
func IsExpectedSendEvidenceError(err error) bool {

more clear name would be: IsEvidenceErrorCausedByResubmition

relayer/processes/xrpl_tx_submitter_test.go line 383 at r1 (raw file):

}

func multiSignTrustSetsOperation(

nit: multiSignTrustSetOperation ?

Reviewable status: 16 of 24 files reviewed, 4 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)

integration-tests/coreum/contract_client_test.go line 367 at r1 (raw file):

Done

integration-tests/coreum/contract_client_test.go line 428 at r1 (raw file):

Removed.

integration-tests/processes/send_from_xrpl_to_coreum_test.go line 159 at r1 (raw file):

Done

relayer/coreum/contract.go line 648 at r1 (raw file):

All.

relayer/processes/amount.go line 18 at r1 (raw file):

Updated comment. Did't get last question.

relayer/processes/amount.go line 54 at r1 (raw file):

Done

relayer/processes/xrpl_tx_observer.go line 171 at r1 (raw file):

Agree, updated.

relayer/processes/xrpl_tx_submitter_test.go line 383 at r1 (raw file):

Done

Reviewed 8 of 8 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)

integration-tests/coreum/contract_client_test.go line 297 at r2 (raw file):

	issuerAcc := chains.XRPL.GenAccount(ctx, t, 0)
	issuer := issuerAcc.String()
	invactiveCurrency := "INA"

should be inactive not invactive

integration-tests/processes/ticket_allocation_test.go line 201 at r2 (raw file):

	// register more than threshold to activate tickets re-allocation
	numberOfXRPLTokensToRegister := int(numberOfTicketsToAllocate) - 1

maybe change this to UsedTicketsThreshold+1 to be more accurate with comment

relayer/coreum/contract.go line 658 at r2 (raw file):

	}

	return nil, nil //nolint:nilnil // is token not found we return nil instead of an error

if token is not found

relayer/coreum/contract.go line 902 at r2 (raw file):

}

// IsXRPLTokenNotEnabledError returns true if error is `XRPLTokenNotEnabled` error.

you can remove the last error from comment.

relayer/processes/amount.go line 35 at r2 (raw file):

		return sdkmath.NewIntFromBigInt(xrplRatAmount.Num()), nil
	}
	// not XRP value is repressed as value multiplied by 10^15

I suggest keeping scientific notation everywhere as a standard (like in spec). Use 1e15 (instead of 10^15) notation everywhere. I will double check the contract for this as well.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)

integration-tests/coreum/contract_client_test.go line 297 at r2 (raw file):

Done

integration-tests/processes/ticket_allocation_test.go line 201 at r2 (raw file):

Agree, done.

relayer/coreum/contract.go line 658 at r2 (raw file):

Done

relayer/coreum/contract.go line 902 at r2 (raw file):

Done.

relayer/processes/amount.go line 35 at r2 (raw file):

Done

Reviewed 5 of 8 files at r2, 8 of 8 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @miladz68, and @wojtek-coreum)

integration-tests/processes/ticket_allocation_test.go line 214 at r3 (raw file):

	runnerEnv.AwaitNoPendingOperations(ctx, t)

	availableTicketsAfterReAllocation, err := runnerEnv.ContractClient.GetAvailableTickets(ctx)

nit: Reallocation single word

relayer/processes/amount.go line 18 at r1 (raw file):

// XRPIssuer is XRP issuer name used on the coreum.
XRPIssuer = "rrrrrrrrrrrrrrrrrrrrrho"

but this it is XRP issuer address used on XRPL ?

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @miladz68, @wojtek-coreum, and @ysv)

integration-tests/processes/ticket_allocation_test.go line 214 at r3 (raw file):

Done

relayer/processes/amount.go line 18 at r1 (raw file):

Clear now, updated.

Reviewed 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @miladz68, @wojtek-coreum, and @ysv)

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dzmitryhil, @miladz68, and @wojtek-coreum)

relayer/processes/amount.go line 18 at r1 (raw file):

For simplicity we consider that issuer for XRP on XRPL is rrrrrrrrrrrrrrrrrrrrrho and use this value to generate corresponding token on Coreum side. This is done to unify representation & flows for all XRPL originated tokens (XRP and issued tokens) on bridge side.

Reviewable status: 30 of 31 files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

relayer/processes/amount.go line 18 at r1 (raw file):

Updated taken your example.

Reviewed 5 of 6 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68 and @wojtek-coreum)

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68 and @wojtek-coreum)